[Refactor] Extract shared delivery upsert - #6056
Conversation
|
@oforiwaasam is attempting to deploy a commit to the agenta projects Team on Vercel. A member of the Team first needs to authorize it. |
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository YAML (base), Organization UI (inherited) Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Summary
write_delivery,write_subscription_delivery_if_live(both inapi/oss/src/dbs/postgres/triggers/dao.py), andSessionStreamsDAO.claim_trigger_delivery(api/oss/src/dbs/postgres/sessions/streams/dao.py) each independently rebuild the same column-values dict,index_elements, andindex_wherefor the delivery upsert. The conflict target and the skipped-column rules can drift between the three copies without any test catching it.Changes
Added a new helper module:
api/oss/src/dbs/postgres/triggers/upsert_utils.pythat centralizes building the trigger-deliveryvaluesdict and theindex_elements/index_whereconflict target. Replaced the duplicated logic inTriggersDAO.write_delivery,TriggersDAO.write_subscription_delivery_if_live, andSessionStreamsDAO.claim_trigger_deliveryso they call the helper. No behavior changes were made.Testing
Verified locally
To ensure testing efficiency, I first ran
uv run --no-sync python run-tests.py oss/tests/pytest/integration/sessions/test_trigger_session_claim_postgres.pyto verify the tests were running correctly before adding my refactoring changes, and then re-verified those same tests after making my changes. See screenshots below:Test Results BEFORE changes
Test results logs
Generated test report
Test Results AFTER changes
Generated test report
Added or updated tests
N/A
Validation
Note on linting:
ruff check --fixrun at the repo root currently reformats ~1,000+ files unrelated to this change (project-wideOptional/List→X | None/list[...]modernization, import reordering, and removal of the#spacer-comment convention used throughout dao.py files). I did not apply those unrelated changes to keep this diff scoped to the#5934refactor. Ranruff format/ruff check --fixonly against the newupsert_utils.py; the two edited DAO files were hand-verified against the existing house style instead.QA follow-up
N/A
Demo
N/A, not a UI change (only refactoring)
Checklist
Contributor Resources
Issue
Closes #5934